-
Notifications
You must be signed in to change notification settings - Fork 40
Make SCIM user table migrations opt-in instead of auto-loading #154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make SCIM user table migrations opt-in instead of auto-loading #154
Conversation
Co-authored-by: arietimmerman <[email protected]>
| If you need to add the `formatted` and `active` columns to your users table, publish the migrations: | ||
|
|
||
| ```bash | ||
| php artisan vendor:publish --tag=laravel-scim-migrations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Make sure to apply these when building/running the Docker image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the Dockerfile to publish the package migrations during the build process. The migration file that was manually created has been replaced with php artisan vendor:publish --tag=laravel-scim-migrations which will publish the migration from the package. (commit 0d9e2fd)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot It seems that by removing the migration from the Dockerfile, the existing migration file does not introduce all required files (such as roles). Please check if something is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the roles field to the migration. The package migration now includes formatted, active, and roles fields. Note: displayName was in the old Dockerfile migration but isn't actually used for Users in the SCIM schema (only for Groups), so it wasn't added. (commit a6da5ec)
Co-authored-by: arietimmerman <[email protected]>
Co-authored-by: arietimmerman <[email protected]>
Co-authored-by: arietimmerman <[email protected]>
Co-authored-by: arietimmerman <[email protected]>
The package automatically ran migrations adding SCIM fields to the users table on installation. Not all SCIM implementations require these fields in their database schema.
Changes
loadMigrationsFrom()withpublishes()using taglaravel-scim-migrationsrolesfield to the package migration (previously only includedformattedandactive)Usage
Users who need these columns can opt-in:
The migration includes:
formatted- User's full display nameactive- User active statusroles- JSON array of user rolesDocker
The Docker image now uses the package migrations:
Existing installations are unaffected. Test suite compatibility maintained (TestCase manually adds columns).
Warning
<issue_title>Migration adding
formattedandactivefields to user model is always included</issue_title><issue_description>Commit daae495 added a migration file
database/migrations/2021_01_01_000003_add_scim_fields_to_users_table.php.The migration is always executed by laravel whenever this package is installed. I wonder if this should be the default behavior, as the real schema does not always need to match the scim schema and not every application require the
formattedfield.Is there some way to fix #143 and not include this migration by default?</issue_description>
Comments on the Issue (you are @copilot in this section)
formattedandactivefields to user model is always included #151Original prompt
formattedandactivefields to user model is always included #151✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.